Fix refresh not working for React-based views#2537
Fix refresh not working for React-based views#2537pjeby wants to merge 1 commit intoblacksmithgu:masterfrom
Conversation
The existing code had a race condition that let it get into a state where no further updates could take place. This revision ensures that the view remains subscribed so long as the container element remains unchanged. (It also removes the extra initialization state and lastReload state, relying on the fact that the refreshOperation is only triggered when a view transitions from hidden to visible, or when the index is changed.) This should reduce the number of React refreshes, as well as the overhead of repeatedly subscribing (and then unsubscribing) to the various events.
|
You state that this applies to react-based views, could you please examplify which views we're talking about? Are this all the dataview and/or dataviewjs views? Does it include the inline variants? Does it apply to both live preview and reading mode? Do you know if this happens to affect #2366 and/or #1734 by any chance? (This is not a requirement before accepting this PR, it's more of checking whether this possibly fixed that issue in addition to other stuff) |
|
The specific behavior I saw was that regular As I experimented with it, I noticed that if I made it so that the setLastReload did not happen until after the updateState was finished, then it didn't stop refreshing. Further experimentation showed the last-reload machinery wasn't actually doing anything useful, as it appeared to be a hold-over from when views refreshed on a set interval rather than in response to an event indicating the index changed. (So I removed it entirely -- an earlier version of DV had the setLastReload but it didn't include the lastReload id in the useEffect deps, so it didn't have the bug AFAICT.) With regard to inline views, I don't think they would be affected since the function being changed here is only used by task, table, and list views, at least so far as I know. If those views are generated by a dataviewjs view, then they might also be affected unless the entire dataviewjs block is also rerun (which presumably it should be). As to live preview vs. reading, I saw the behavior in both. On switching from one to the other in a given pane, the view would refresh on exactly one change to the index, and then stop refreshing afterward. If you want to try to repro/test for yourself, I'd suggest making a query that depends on sorting notes by last-modified timestamp in descending order, with a limit. Then modify the second or third note in the list (by typing in it), and see if it jumps to the top of the list. The behavior I saw was that it would do that the first time, but if you then went to the new second or third note and edited it, it would not jump to the top unless you recreated the rendered dataview (by switching edit/read modes, or moving the cursor into the dataview block and back out). With the change in this PR, it would consistently update upon editing any note. |
2 participants
The existing code had a race condition that let it get into a state where no further updates could take place. This revision ensures that the view remains subscribed so long as the container element remains unchanged. (It also removes the extra initialization state and lastReload state, relying on the fact that the refreshOperation is only triggered when a view transitions from hidden to visible, or when the index is changed.)
This should reduce the number of React refreshes, as well as the overhead of repeatedly subscribing (and then unsubscribing) to the various events.